Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helpx_Milo Components Script Fixes #235

Merged
merged 11 commits into from
Jan 8, 2024
Merged

Conversation

kirupacommit
Copy link
Collaborator

No description provided.

@@ -35,11 +34,11 @@ const config = {
workers: process.env.CI ? 2 : undefined,
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: process.env.CI
? [['github'], ['list'], ['../utils/reporters/base-reporter.js']]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add base-reporter.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JackySun9 I appreciate your comments. Added Base Reporter along with d5eb7a1.
image

@@ -4,8 +4,26 @@ module.exports = {
{
tcid: '0',
name: '@draft',
path: '/automation/blocks/Drafts/draft-test',
path: '/automation/blocks/Draft/draft-test.html',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Recently I made some folder changes.
image

@skumar09
Copy link
Contributor

skumar09 commented Jan 4, 2024

@kirupacommit : Helpx daily run is failing because of the test title

image

Your test title does not have name and tag information.

image

Please refer to the Nala test template

image

The test title should contain the test name and tag pieces of information that are defined in the corresponding .spec.js

image

@skumar09 skumar09 self-requested a review January 4, 2024 21:18
@@ -35,11 +34,11 @@ const config = {
workers: process.env.CI ? 2 : undefined,
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: process.env.CI
? [['github'], ['list'], ['../utils/reporters/base-reporter.js']]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirupacommit :
Any reason not using base-reporter.js , we need this for dailyrun report metrics

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now i have added Base reporter to the helpx Project

@@ -77,4 +80,4 @@ const config = {
},
],
};
export default config;
export default helpxconfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirupacommit : Any reason for changing this from default config?, as other dependent libraries, use default config

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought of giving project specific config name so used helpxconfig, since other libs are using config i have changed to default config.

@kirupacommit
Copy link
Collaborator Author

@skumar09 Thanks for your insight on the Github Actions , Now i have renamed the test cases and triggered the jobs as well. Thank You!

@skumar09
Copy link
Contributor

skumar09 commented Jan 5, 2024

@kirupacommit :

  • Approving the PR, all looks good. Thanks for the discussion and adding dailyrun.yaml. for Helpx, please start monitoring the slack channel: milo-automation-dev for daily run results.
  • Please update the few locators in a better way ( refer to procedure pom file )

baseURL,
}) => {
console.info(`[Test Page]: ${baseURL}${features[0].path}`);
test(`verify Code Block Page Elements`, async ({baseURL}) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the test title as per the Nala test title template.

this.thirdStepLink = page.locator("//div[contains(@daa-lh,'b11|procedure|nopzn|nopzn')]/ol/li[3]/div/strong/a[contains(@href,'https://account.adobe.com/plans')]");
this.iconinprocedureComponet = page.locator("//div[contains(@daa-lh,'b11|procedure|nopzn|nopzn')]/ol/li[6]/div/ol/li/span[contains(@class,'icon icon-objectives margin-left margin-right')]");
this.notealertinprocedure = page.locator("//div[contains(@daa-lh,'b11|procedure|nopzn|nopzn')]/ol/li[6]/child::div/child::div[contains(@class,'note alert')]");
this.noteiconinprocedure = page.locator("//div[contains(@daa-lh,'b11|procedure|nopzn|nopzn')]/ol/li[6]/child::div/child::div[contains(@class,'note alert')]/span[contains(@class,'note-icon')]");
Copy link
Contributor

@skumar09 skumar09 Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I would not suggest using such a lengthy locator ( i.e., Xpath)
  • Playwright provides plenty of other better options; please try using better selectors like CSS ( please refer to playwright locators
  • daa-lh,'b11|procedure|nopzn|nopzn are personalization attributes

@skumar09 skumar09 merged commit 7945139 into adobecom:main Jan 8, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants